Skip to content

Conversation

@SomeoneToIgnore
Copy link
Contributor

Part of https://rust-lang.zulipchat.com/#narrow/stream/185405-t-compiler.2Fwg-rls-2.2E0/topic/autoimport.20weirdness

Removes all flyimport completions for any unqualified associated type, effectively reverting #8095
I've explained the reasoning in the corresponding FIXME and open to discussions.
As an alternative way, we could add yet another parameter in the method that's used by the qualify_path and enable it for the qualify assists only.

@SomeoneToIgnore
Copy link
Contributor Author

@Veykril , could you have a look?

There are ways to return the existing bits of "qualify the assoc item" functionality if you're interested in it now, but it'll work for the current crate's items only.

@flodiebold
Copy link
Member

Hmm, we don't necessarily need to get rid of all associated item completions. Regarding the issue in the Zulip thread, IMO it'd also be fine to complete ToChalk::from_chalk; the problem is that there's a suggestion for each impl of the trait.

@SomeoneToIgnore
Copy link
Contributor Author

SomeoneToIgnore commented Mar 20, 2021

But ToChalk::from_chalk won't compile without us explicitly specifying which impl to use?
https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=e717f2e134d2936e7ae0243f13e9928b

That was my reasoning for excluding such cases at least: we cannot guess which impl to use without the user helping us and having just the trait is not enough.

We could instead propose the corresponding structs and their assoc items, but that's unavailable for a number of reasons, ergo also excluded and the test ignored.

@Veykril
Copy link
Member

Veykril commented Mar 20, 2021

I personally don't see too much of a problem in not seeing associated items I think, at least for now? I don't think I can tell until I'm missing it though 😄 It would certainly be nice to at least have it for non-trait impls but if I saw that right reading the comment that will take quite a bit of work to be doable efficiently.

@flodiebold
Copy link
Member

flodiebold commented Mar 21, 2021

But ToChalk::from_chalk won't compile without us explicitly specifying which impl to use?

https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=71fd67cdf4b5d7520420af4475a71824 🙂
You're using this all the time when writing Default::default().
I'm not totally convinced that we should have this either, though. (Actually I'd probably personally prefer not having them.)

@SomeoneToIgnore
Copy link
Contributor Author

I'm actually following some of the pedantic clippy rules and always write Struct::default() 🙂
But it's a great example, now I understand where else the qualifier can come from, thank you.

I'm also not sure we should have this kinds of completions, not right away at least: it feels like I have to write way more code and tests to properly support such cases, so let's prohibit them for a while and iterate on top.

bors r+

@SomeoneToIgnore
Copy link
Contributor Author

changelog fix

@bors
Copy link
Contributor

bors bot commented Mar 21, 2021

@bors bors bot merged commit 09412d8 into rust-lang:master Mar 21, 2021
@SomeoneToIgnore SomeoneToIgnore deleted the less-assoc-items branch March 21, 2021 09:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants